Insulate bitcoin-ohttp and bitcoin-hpke from the public API#1702
Draft
spacebear21 wants to merge 2 commits into
Draft
Insulate bitcoin-ohttp and bitcoin-hpke from the public API#1702spacebear21 wants to merge 2 commits into
spacebear21 wants to merge 2 commits into
Conversation
HpkeSecretKey and HpkePublicKey each wrapped a bitcoin-hpke KEM key type behind a public tuple field plus a Deref impl, leaking those foreign types into payjoin's public API. A breaking release of bitcoin-hpke would then force a breaking release of payjoin. Make the inner fields private and drop the Deref impls so the foreign key types are no longer reachable through the public API. Callers use the existing accessor methods and to_/from_compressed_bytes, so no downstream code changes are required.
Several bitcoin-ohttp types leaked into payjoin's public API: OhttpKeys exposed its inner ohttp::KeyConfig through a public field and Deref, decode/to_bytes returned ohttp::Error, and ohttp::ClientResponse appeared directly in the v2 create_*/process_* signatures. A breaking release of bitcoin-ohttp would then force a breaking release of payjoin. Make the OhttpKeys field private and drop its Deref/DerefMut, wrap the key config error in a new OhttpKeysError that erases ohttp::Error, and return the OHTTP response context as a new opaque OhttpResponse newtype. ohttp_encapsulate now takes &OhttpKeys directly. Tests build OhttpKeys through the public decode API via a new payjoin_test_utils::ohttp_key_config_bytes helper (plus ohttp_server for the emulated gateway), since production code only ever constructs OhttpKeys from directory bytes.
Collaborator
Coverage Report for CI Build 28557993858Coverage increased (+0.04%) to 85.781%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Member
|
concept ACK this is important before 1.0-rc.4 |
Collaborator
|
We either need to remove |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of the 1.0 API hardening: remove pre-1.0 foreign crate types
(
bitcoin-ohttp0.6,bitcoin-hpke0.13) from payjoin's public API so that abreaking release of either dependency no longer forces a breaking release of
payjoin. The rust-bitcoin 0.32 and bitcoin_uri 0.1 pins are intentionally kept
for 1.0.
bitcoin-hpke
Make the inner tuple fields of
HpkeSecretKeyandHpkePublicKeyprivate anddrop their
Derefimpls, so the wrapped secp256k1/hpke KEM key types are nolonger reachable through the public API. Callers use the existing accessors and
to_/from_compressed_bytes.bitcoin-ohttp
OhttpKeys' innerohttp::KeyConfigfield is now private and theDeref/DerefMutimpls are removed.OhttpKeys::decode/encode/to_bytes/TryFromreturn a new opaqueOhttpKeysErrorinstead ofohttp::Error(the foreign error is erasedbehind
Box<dyn Error + Send + Sync>, keeping thesource()chain).create_*/process_*methods now return/accept a new opaqueOhttpResponsenewtype instead ofohttp::ClientResponse.ohttp_encapsulatetakes&OhttpKeysdirectly.payjoin-cliandpayjoin-ffiare updated to the new API. The FFI's exporteduniffi surface is unchanged (only its internal plumbing types), so the language
bindings do not require regeneration.
The two changes are split into separate commits (hpke, then ohttp) for review.
Disclosure: co-authored by Claude Code (Opus 4.8)
🤖 Generated with Claude Code